chore(storage): support PartSizeHint and optimize cleanup#19930
chore(storage): support PartSizeHint and optimize cleanup#19930cpriti-os wants to merge 4 commits into
Conversation
product-auto-label
Bot
added
the
api: storage
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request renames PartSize to PartSizeHint in ParallelUploadConfig, increases the minimum part size from 5 MiB to 8 MiB, and updates the associated tests. It also optimizes cleanup by enabling automatic deletion of source objects during compose operations (DeleteSourceObjects = true) and updating the local cleanup logic to run only when an upload fails and the final compose does not succeed. Feedback was provided regarding the background cleanup goroutine, which currently uses the potentially canceled parent context. To prevent temporary object leaks when a context is canceled, it is recommended to use context.WithoutCancel for the cleanup operation.
| defer func() { | ||
| go s.doCleanupFn(s) | ||
| s.mu.Lock() | ||
| hasErr := s.firstErr != nil | ||
| finalSucceeded := s.finalComposeSucceeded | ||
| s.mu.Unlock() | ||
| if hasErr && !finalSucceeded { | ||
| go s.doCleanupFn(s) | ||
| } | ||
| }() |
There was a problem hiding this comment.
When the parent context is canceled, the background cleanup goroutine go s.doCleanupFn(s) will run using the canceled context s.ctx. This causes all subsequent GCS API calls (such as deleting temporary objects) to fail immediately with context.Canceled, resulting in leaked temporary objects in the bucket.
To prevent this, use context.WithoutCancel to ensure the cleanup operation is not prematurely terminated when the parent context is canceled.
| defer func() { | |
| go s.doCleanupFn(s) | |
| s.mu.Lock() | |
| hasErr := s.firstErr != nil | |
| finalSucceeded := s.finalComposeSucceeded | |
| s.mu.Unlock() | |
| if hasErr && !finalSucceeded { | |
| go s.doCleanupFn(s) | |
| } | |
| }() | |
| defer func() { | |
| s.mu.Lock() | |
| hasErr := s.firstErr != nil | |
| finalSucceeded := s.finalComposeSucceeded | |
| s.mu.Unlock() | |
| if hasErr && !finalSucceeded { | |
| s.mu.Lock() | |
| s.ctx = context.WithoutCancel(s.ctx) | |
| s.mu.Unlock() | |
| go s.doCleanupFn(s) | |
| } | |
| }() |
References
- When a cleanup function must be attempted even if its parent context is canceled, use
context.WithoutCancelto ensure the operation is not prematurely terminated.
2 participants
No description provided.